Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix missing icons on overflowset v7 #26798

Open
wants to merge 1 commit into
base: 7.0
Choose a base branch
from
Open

Fix missing icons on overflowset v7 #26798

wants to merge 1 commit into from

Conversation

advancewarsbest
Copy link
Contributor

Similar as recently submitted PR but for version 7.X

From the developer examples of fluent UI for React, the overflowset custom examples overflow icons are not displaying, this PR resolves it.

tested using the codepen link from the website.

Web page in question link v7:
https://developer.microsoft.com/en-us/fluentui?fabricVer=7#/controls/web/overflowset

Previous Behavior

the React FluentUI overflowset example does not show the icons that are in the code

New Behavior

Resolves the missing icon issues

Related Issue(s)

None

@advancewarsbest
Fix missing icons on overflowset
615f2aa 
This PR is for Fluent UI React 7.X

The overflowset custom examples overflow icons are not showing up, this resolves that.

tested using the codepen link from the website.

https://developer.microsoft.com/en-us/fluentui?fabricVer=8#/controls/web/overflowset
@msft-fluent-ui-bot msft-fluent-ui-bot added the needs cherry-pick Temporary label for PRs which may need to be cherry-picked to master label Feb 10, 2023
msft-fluent-ui-bot
msft-fluent-ui-bot previously requested changes Feb 10, 2023
View reviewed changes
Copy link
Collaborator

@msft-fluent-ui-bot msft-fluent-ui-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this change to the 7.0 branch may not have been submitted to master yet. Now that version 8 has released, all changes must be submitted to the master branch first (except in emergencies or if the change is irrelevant to version 8).

Please do one of the following:

  • If you've already created a PR to master, add a link to it
  • If the change is irrelevant to version 8, add a comment explaining why
  • Otherwise, create a PR to master with this same change, and add a link to it

After that, you can dismiss this review and remove the "needs cherry-pick" label (or ask a team member to help do so).

Want to avoid this in the future? Include text like "Cherry-pick of ####" in the PR description (where #### is the real master PR number).

@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 615f2aa:

Sandbox Source
Fluent UI Button Configuration
codesandbox-react-template Configuration

@advancewarsbest advancewarsbest changed the title Fix missing icons on overflowset Fix missing icons on overflowset v7 Feb 10, 2023
@fabricteam
Copy link
Collaborator

fabricteam commented Feb 10, 2023 •
edited
Loading

🕵 FluentUI-v7 Open the Visual Regressions report to inspect the 31 screenshots

✅ There was 0 screenshots added, 0 screenshots removed, 1342 screenshots unchanged, 0 screenshots with different dimensions and 31 screenshots with visible difference.

unknown 31 screenshots
Image Name Diff(in Pixels) Image Type
Card.Horizontal Card - Example with contents.default.chromium.png 27464 Changed
Card.Horizontal Card - Example with contents.focus.chromium.png 27376 Changed
Card.Horizontal Card - Example with contents.hover.chromium.png 27355 Changed
Card.Vertical Card - Example with contents - Image in middle.default.chromium.png 8094 Changed
Card.Vertical Card - Example with contents - Image in middle.focus.chromium.png 8035 Changed
Card.Vertical Card - Example with contents - Image in middle.hover.chromium.png 8094 Changed
Card.Vertical Card - Example with contents - Image on top.default.chromium.png 277566 Changed
Card.Vertical Card - Example with contents - Image on top.focus.chromium.png 275241 Changed
Card.Vertical Card - Example with contents - Image on top.hover.chromium.png 277514 Changed
Image.Fit- CenterContain, image larger.default.chromium.png 12593 Changed
Image.Fit- CenterContain, image smaller.default.chromium.png 32236 Changed
Image.Fit- CenterContain, image taller.default.chromium.png 18134 Changed
Image.Fit- CenterContain, image wider.default.chromium.png 18134 Changed
Image.Fit- center, image larger.default.chromium.png 11026 Changed
Image.Fit- center, image smaller.default.chromium.png 2008 Changed
Image.Fit- centerCover, image larger.default.chromium.png 17710 Changed
Image.Fit- centerCover, image smaller.default.chromium.png 32236 Changed
Image.Fit- centerCover, image taller.default.chromium.png 24267 Changed
Image.Fit- centerCover, image wider.default.chromium.png 24267 Changed
Image.Fit- contain, image taller.default.chromium.png 1171 Changed
Image.Fit- contain, image wider.default.chromium.png 3471 Changed
Image.Fit- cover, image taller.default.chromium.png 7613 Changed
Image.Fit- cover, image wider.default.chromium.png 7613 Changed
Image.Fit- none, image larger.default.chromium.png 10967 Changed
Image.Fit- none, image smaller.default.chromium.png 2008 Changed
Image.Maximize frame, landscape container.default.chromium.png 4074 Changed
Image.No fit, no w-h.default.chromium.png 84392 Changed
Image.No fit, only height.default.chromium.png 21927 Changed
Image.No fit, only width.default.chromium.png 166761 Changed
TeachingBubble.WideIllustration - RTL.default.chromium.png 25170 Changed
TeachingBubble.WideIllustration.default.chromium.png 25234 Changed

@fabricteam
Copy link
Collaborator

fabricteam commented Feb 10, 2023 •
edited
Loading

Perf Analysis

No significant results to display.

All results

Scenario Render type 7.0 Ticks PR Ticks Iterations Status
BaseButton mount 1131 1143 5000
Breadcrumb mount 35025 35730 5000
Checkbox mount 1645 1617 5000
CheckboxBase mount 1440 1424 5000
ChoiceGroup mount 4400 4429 5000
ComboBox mount 1056 1058 1000
CommandBar mount 6860 6946 1000
ContextualMenu mount 10873 11624 1000
DefaultButton mount 1314 1302 5000
DetailsRow mount 3360 3367 5000
DetailsRowFast mount 3374 3444 5000
DetailsRowNoStyles mount 3243 3233 5000
Dialog mount 2131 2137 1000
DocumentCardTitle mount 1887 1898 1000
Dropdown mount 2362 2351 5000
FocusTrapZone mount 1811 1767 5000
FocusZone mount 1876 1865 5000
GroupedList mount 804 864 2
GroupedListV2 mount 485 483 2
IconButton mount 1734 1731 5000
Label mount 681 680 5000
Layer mount 1977 1978 5000
Link mount 778 770 5000
MenuButton mount 1544 1547 5000
MessageBar mount 2108 2144 5000
Nav mount 2980 2969 1000
OverflowSet mount 1559 1540 5000
Panel mount 1532 1531 1000
Persona mount 1177 1186 1000
Pivot mount 1481 1469 1000
PrimaryButton mount 1427 1431 5000
Rating mount 6999 6983 5000
SearchBox mount 1417 1407 5000
Shimmer mount 2608 2642 5000
Slider mount 1633 1647 5000
SpinButton mount 4269 4269 5000
Spinner mount 761 759 5000
SplitButton mount 2793 2809 5000
Stack mount 832 823 5000
StackWithIntrinsicChildren mount 1727 1734 5000
StackWithTextChildren mount 4611 4601 5000
SwatchColorPicker mount 8417 8431 5000
TagPicker mount 2479 2470 5000
TeachingBubble mount 41376 41525 5000
Text mount 773 762 5000
TextField mount 1509 1494 5000
Toggle mount 1099 1105 5000
button mount 457 451 5000

@advancewarsbest
Copy link
Contributor Author

It looks like this change to the 7.0 branch may not have been submitted to master yet. Now that version 8 has released, all changes must be submitted to the master branch first (except in emergencies or if the change is irrelevant to version 8).

Please do one of the following:

  • If you've already created a PR to master, add a link to it
  • If the change is irrelevant to version 8, add a comment explaining why
  • Otherwise, create a PR to master with this same change, and add a link to it

After that, you can dismiss this review and remove the "needs cherry-pick" label (or ask a team member to help do so).

Want to avoid this in the future? Include text like "Cherry-pick of ####" in the PR description (where #### is the real master PR number).

This change is irrelevant to version 8, but I do not seem to have the ability to remove the cherry pick label for this PR to be approved by the fluent-ui-bot, @micahgodbolt - can you kindly remove the cherry pick label for this PR so it can be approved by the bot due to its comment. Thank you in advance!

@advancewarsbest
Copy link
Contributor Author

It looks like this change to the 7.0 branch may not have been submitted to master yet. Now that version 8 has released, all changes must be submitted to the master branch first (except in emergencies or if the change is irrelevant to version 8).
Please do one of the following:

  • If you've already created a PR to master, add a link to it
  • If the change is irrelevant to version 8, add a comment explaining why
  • Otherwise, create a PR to master with this same change, and add a link to it

After that, you can dismiss this review and remove the "needs cherry-pick" label (or ask a team member to help do so).
Want to avoid this in the future? Include text like "Cherry-pick of ####" in the PR description (where #### is the real master PR number).

This change is irrelevant to version 8, but I do not seem to have the ability to remove the cherry pick label for this PR to be approved by the fluent-ui-bot, @micahgodbolt - can you kindly remove the cherry pick label for this PR so it can be approved by the bot due to its comment. Thank you in advance!

Accident apologies - accidentally closed the PR

@micahgodbolt micahgodbolt removed the needs cherry-pick Temporary label for PRs which may need to be cherry-picked to master label Feb 15, 2023
@micahgodbolt micahgodbolt removed their assignment Feb 21, 2023
@micahgodbolt micahgodbolt reopened this Feb 21, 2023
@micahgodbolt micahgodbolt self-assigned this Feb 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@micahgodbolt micahgodbolt micahgodbolt approved these changes

@msft-fluent-ui-bot msft-fluent-ui-bot Awaiting requested review from msft-fluent-ui-bot

Assignees

@micahgodbolt micahgodbolt

Labels
Area: Website Fabric 7
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@advancewarsbest @fabricteam @micahgodbolt @msft-fluent-ui-bot